Setup TagField to work within AssetAdmin (Fixes #107) #120

wilr · 2018-10-10T02:36:09Z

Form field now appears, can be added and searched.

This also fixes missing label

client/src/components/TagField.js

src/TagField.php

robbieaverill · 2018-11-15T21:05:32Z

I think this PR mostly looks good, but there's a couple of breaking changes that would need to be adjusted and polished. Also I've just made a PR at #122 which will cause a couple of conflicts with adding the same logic in this PR in places

wilr · 2018-11-15T21:09:03Z

Happy if @robbieaverill you want to incorporate the bits needed into #122 and close this.

robbieaverill · 2018-11-15T21:15:27Z

@wilr I might try and do it on bug day tomorrow

robbieaverill · 2018-11-16T10:32:36Z

(Context: #122 with these changes applied)

Hrmmm, I've got the fields rendering correctly in asset admin but when I have 'name' => $this->getName() . '[]'; redux-form won't validate or store the tag values correctly, which means (A) it won't add the tag in the UI and (B) it won't save it (related issue: silverstripe/silverstripe-admin#639)

If I use a regular $this->getName(), tags end up getting pushed into the DOM in multiple hidden inputs with the same name.

Does this PR actually save @wilr?

@ScopeyNZ are you able to provide any advice here in terms of how this field interacts with redux-form? Again related: silverstripe/silverstripe-admin#639

wilr · 2018-11-16T18:54:00Z

@robbieaverill Yep saves perfectly, been running in prod for a couple weeks now. But this is using composer.lock (https://gist.github.com/wilr/68ed4e6eb9f063da4e00dec6831e0a98). Note the [] was to copy how CheckboxSetField passed its' array.

robbieaverill · 2018-11-19T16:18:24Z

@wilr In this PR I've noticed that I get an undefined index error when using the fields on a non-React form, and that StringTagField isn't working in the React context yet. It does look like this PR does make TagField work correctly in a React context, so thank you for that! I'll try and work out why #122 isn't based off of this and move it to that.

robbieaverill · 2018-11-19T17:17:39Z

Yeah OK, I can get it working in either entwine or React context but not both at the same time (this PR is the same). I've raised an issue at silverstripe/silverstripe-admin#755 for tracking. I suspect we'll have to refactor more of the logic in TagField to get this to work, and probably do as you've suggested here and switch to extending MultiSelectField.

I'll leave this PR open

ScopeyNZ · 2019-07-22T04:29:29Z

It took me wayyyy too long but I finally got around to having a go at fixing this. I've tested this using the blog module and using elemental in-line editing to render a tag field in the React context.

For the react context you'll need the admin PR merged as a dependency (silverstripe/silverstripe-admin#912) otherwise the values won't appear in the field.

robbieaverill · 2019-07-22T09:43:28Z

@ScopeyNZ is that the only dependency for this PR, e.g. once the admin PR is merged will this PR fix the original issue?

ScopeyNZ · 2019-07-22T21:46:39Z

Yeah we spoke about this a little offline. The admin PR ensures that the field value is correctly set from redux state. So I think this only matters in the elemental context.

ScopeyNZ

"Reviewing" so that I can stop being a requested reviewer 😅

NightJar

When viewing a BlogPost and creating tags, everything works fine.
On saving a BlogPost with new tags, everything works fine.
On saving a BlogPost with existing tags, everything is not fine.

Current tags are removed from the relationship
An equal number of new tags are created and assigned to the relationship
These new tags have the Title and URLSegment of the ID field of the tags they're replacing.

Reproduction

Create a BlogPost
Assign tags "one" and "two" (two separate tags).
Save - See "one" and "two" assigned.
Reload - see "one" and "two" assigned.
Save - see "1" and "2" assigned.
Save - see "3" and "4" assigned.
Save - see "5" and "6" assigned.
Reload - see "5" and "6" assigned.

Environment

    "repositories": [
        {
            "type": "vcs",
            "url": "[email protected]:creative-commoners/silverstripe-admin"
        },
        {
            "type": "vcs",
            "url": "https://github.com/wilr/silverstripe-tagfield/"
        }
    ],
    "require": {
        "silverstripe/recipe-plugin": "^1",
        "silverstripe/recipe-cms": "^4.4",
        "dnadesign/silverstripe-elemental": "^4.3",
        "silverstripe/elemental-fileblock": "^2.0",
        "silverstripe/elemental-bannerblock": "^2.0",
        "silverstripe/blog": "^3.3",
        "silverstripe/admin": "dev-pulls/1.4/field-names-in-disarray as 1.4.1",
        "silverstripe/tagfield": "dev-fixes/107-tagfield-in-admin as 2.3.0"
    },

Testing as per #107 OP seems to work fine. Tags do not appear to "duplicate" themselves.

ScopeyNZ · 2019-07-24T03:42:25Z

Thanks @NightJar . There was some confusion about IDs vs label titles. Should be squared away now.

robbieaverill

Some small questions. Let's also make this a minor release rather than a patch, because we're adding some new APIs, replacing the react-select library with a newer one, and changing the API response structure

client/src/components/TagField.js

client/src/components/TagField.scss

src/TagField.php

NightJar · 2019-07-24T21:27:17Z

Thanks @ScopeyNZ - can confirm latest update has resolved that issue :)

Issues identified were fixed.

This also fixes an issue where tags would be created and tags could not have the same name as the ID of an existing tag And the react-select module is updated to the (current) latest 1.x version And the tests are now working again after updating jest

ScopeyNZ · 2019-07-25T00:28:38Z

Ok I've addressed feedback and rebased for a minor release. Thanks for the review @robbieaverill & @NightJar!

robbieaverill · 2019-07-25T07:04:31Z

Good result everyone!

wilr mentioned this pull request Oct 10, 2018

TagField doesn't render in AssetAdmin #107

Closed

wilr changed the title ~~WIP: Setup TagField to work within AssetAdmin (Fixes #107)~~ Setup TagField to work within AssetAdmin (Fixes #107) Oct 10, 2018

robbieaverill reviewed Oct 10, 2018

View reviewed changes

client/src/components/TagField.js Outdated Show resolved Hide resolved

robbieaverill reviewed Oct 10, 2018

View reviewed changes

client/src/components/TagField.js Outdated Show resolved Hide resolved

src/TagField.php Outdated Show resolved Hide resolved

src/TagField.php Outdated Show resolved Hide resolved

robbieaverill requested a review from ScopeyNZ October 10, 2018 09:12

robbieaverill added the affects/v4 label Nov 15, 2018

robbieaverill self-assigned this Nov 16, 2018

robbieaverill removed their assignment Nov 16, 2018

robbieaverill self-assigned this Nov 19, 2018

robbieaverill mentioned this pull request Nov 19, 2018

FIX TagField and StringTagField now work correctly on SilverStripe 4.3 #122

Merged

robbieaverill removed their assignment Nov 19, 2018

ScopeyNZ mentioned this pull request Feb 9, 2019

SilverStripe TagField doesn't appear in Block Settings silverstripe/silverstripe-elemental#599

Closed

ScopeyNZ self-assigned this Mar 7, 2019

robbieaverill added the feedback-required/core-team label Apr 29, 2019

ScopeyNZ changed the base branch from master to 2.3 July 22, 2019 04:27

ScopeyNZ reviewed Jul 23, 2019

View reviewed changes

ScopeyNZ added change/patch and removed feedback-required/core-team labels Jul 23, 2019

ScopeyNZ removed their assignment Jul 23, 2019

NightJar previously requested changes Jul 23, 2019

View reviewed changes

robbieaverill reviewed Jul 24, 2019

View reviewed changes

client/src/components/TagField.js Outdated Show resolved Hide resolved

client/src/components/TagField.scss Outdated Show resolved Hide resolved

src/TagField.php Outdated Show resolved Hide resolved

wilr and others added 5 commits July 25, 2019 12:13

Setup TagField to work within AssetAdmin (Fixes #107)

068e4c6

FIX Ensure tagfield is compatible with both React and Entwine contexts

e97a82f

FIX Removing potentially breaking lower-case change (and fix tests)

61d3d04

Code clean-up

bd84139

ScopeyNZ changed the base branch from 2.3 to master July 25, 2019 00:26

Run automated phpcs linting

804c6ba

robbieaverill approved these changes Jul 25, 2019

View reviewed changes

robbieaverill merged commit 58edb12 into silverstripe:master Jul 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup TagField to work within AssetAdmin (Fixes #107) #120

Setup TagField to work within AssetAdmin (Fixes #107) #120

wilr commented Oct 10, 2018 •

edited

Loading

robbieaverill commented Nov 15, 2018 •

edited

Loading

wilr commented Nov 15, 2018

robbieaverill commented Nov 15, 2018

robbieaverill commented Nov 16, 2018

wilr commented Nov 16, 2018

robbieaverill commented Nov 19, 2018

robbieaverill commented Nov 19, 2018

ScopeyNZ commented Jul 22, 2019

robbieaverill commented Jul 22, 2019

ScopeyNZ commented Jul 22, 2019

ScopeyNZ left a comment

NightJar left a comment •

edited

Loading

ScopeyNZ commented Jul 24, 2019

robbieaverill left a comment

NightJar commented Jul 24, 2019

ScopeyNZ commented Jul 25, 2019

robbieaverill commented Jul 25, 2019

Setup TagField to work within AssetAdmin (Fixes #107) #120

Setup TagField to work within AssetAdmin (Fixes #107) #120

Conversation

wilr commented Oct 10, 2018 • edited Loading

robbieaverill commented Nov 15, 2018 • edited Loading

wilr commented Nov 15, 2018

robbieaverill commented Nov 15, 2018

robbieaverill commented Nov 16, 2018

wilr commented Nov 16, 2018

robbieaverill commented Nov 19, 2018

robbieaverill commented Nov 19, 2018

ScopeyNZ commented Jul 22, 2019

robbieaverill commented Jul 22, 2019

ScopeyNZ commented Jul 22, 2019

ScopeyNZ left a comment

Choose a reason for hiding this comment

NightJar left a comment • edited Loading

Choose a reason for hiding this comment

ScopeyNZ commented Jul 24, 2019

robbieaverill left a comment

Choose a reason for hiding this comment

NightJar commented Jul 24, 2019

ScopeyNZ commented Jul 25, 2019

robbieaverill commented Jul 25, 2019

wilr commented Oct 10, 2018 •

edited

Loading

robbieaverill commented Nov 15, 2018 •

edited

Loading

NightJar left a comment •

edited

Loading